-
-
Notifications
You must be signed in to change notification settings - Fork 461
feat(replay): Adding OkHttp Request/Response bodies #4796
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…dcrumbCallback to extract NetworkRequestData from Hint -> NetworkRequestData is landing on DefaultReplayBreadcrumbConverter via Hint.get(replay:networkDetails)
...ry-android-replay/src/main/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverter.kt
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
Outdated
Show resolved
Hide resolved
romtsn
reviewed
Oct 13, 2025
romtsn
reviewed
Oct 13, 2025
sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt
Outdated
Show resolved
Hide resolved
romtsn
reviewed
Oct 13, 2025
sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt
Outdated
Show resolved
Hide resolved
romtsn
reviewed
Oct 13, 2025
...ry-android-replay/src/main/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverter.kt
Outdated
Show resolved
Hide resolved
These get set on the breadcrumb Hint, and then hang around in DefaultReplayBreadcrumbConverter until the replay segment has been sent off See https://github.com/getsentry/sentry-javascript/blob/632f0b953d99050c11b0edafb9f80b5f3ba88045/packages/replay-internal/src/types/performance.ts#L133-L140 https://github.com/getsentry/sentry-javascript/blob/develop/packages/replay-internal/src/types/request.ts#L12
Manually set networkDetailHasUrls to pass this check https://github.com/getsentry/sentry-javascript/blob/d1646c8a281dd8795c5a6a3b8e18f2e7069e7fa9/packages/replay-internal/src/util/handleRecordingEmit.ts#L134 https://github.com/getsentry/sentry/blob/master/static/app/views/replays/detail/network/details/getOutputType.tsx#L33 https://github.com/getsentry/sentry/blob/master/static/app/views/replays/detail/network/details/content.tsx#L55-L56
added some unit tests: ./gradlew :sentry-android-replay:testDebugUnitTest --tests="*DefaultReplayBreadcrumbConverterTest*"
Breadcrumb.java has several timestamp fields: `timestamp: Date`, `timestampMs: Long`, `nanos: Long` `hashcode` was relying solely on `timestamp`, which can be null depending on which constructor was used. => Change to use getTimestamp as 1. this is what equals does (consistency) 2. getTimestamp initialises timestamp if null.
43jay
commented
Oct 24, 2025
...ry-android-replay/src/main/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverter.kt
Show resolved
Hide resolved
43jay
commented
Oct 24, 2025
sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
Outdated
Show resolved
Hide resolved
43jay
commented
Oct 24, 2025
...ry-android-replay/src/main/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverter.kt
Outdated
Show resolved
Hide resolved
Entrypoint is NetworkDetailCaptureUtils (initializeForUrl) called from SentryOkHttpInterceptor - common logic to handle checking sdk options. - Accept data from http client via NetworkBodyExtractor, NetworkHeaderExtractor interfaces that can be reused in future (if needed) Placeholder impl for req/resp bodies. From https://docs.sentry.io/platforms/javascript/session-replay/configuration/ - networkDetailAllowUrls, networkDetailDenyUrls, - networkCaptureBodies - networkRequestHeaders, networkResponseHeaders These SDKOptions don't exist yet => impl acts as if they do, but have not been enabled.
Also got rid of the VisibleForTesting constructor in NetworkRequestData review comment - https://github.com/getsentry/sentry-java/pull/4796/files#r2474350569
This reverts commit 13c1da8.
sentry/src/main/java/io/sentry/util/network/NetworkBodyParser.java
Outdated
Show resolved
Hide resolved
sentry/src/main/java/io/sentry/util/network/NetworkDetailCaptureUtils.java
Show resolved
Hide resolved
review thread - https://github.com/getsentry/sentry-java/pull/4796/files#r2461102218 squash into breadcrumb equals
sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
Show resolved
Hide resolved
romtsn
approved these changes
Nov 6, 2025
Member
romtsn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I guess we don't need a changelog entry yet because we don't expose the options to enable capturing req/resp bodies.
Still needs to rebase with main and the tests that I mentioned would be nice (but can be done in a separate PR too)
Contributor
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| fcec2f2 | 328.91 ms | 387.75 ms | 58.84 ms |
| ee747ae | 382.73 ms | 435.41 ms | 52.68 ms |
| 7314dbe | 437.83 ms | 505.64 ms | 67.81 ms |
| ee747ae | 386.94 ms | 431.43 ms | 44.49 ms |
| 27d7cf8 | 309.43 ms | 364.27 ms | 54.85 ms |
| b77456b | 393.26 ms | 441.10 ms | 47.84 ms |
| 17a0955 | 372.53 ms | 446.70 ms | 74.17 ms |
| 27d7cf8 | 314.17 ms | 347.00 ms | 32.83 ms |
| b750b96 | 408.98 ms | 480.32 ms | 71.34 ms |
| d217708 | 355.34 ms | 381.39 ms | 26.05 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| fcec2f2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 7314dbe | 1.58 MiB | 2.10 MiB | 533.45 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 27d7cf8 | 1.58 MiB | 2.12 MiB | 549.42 KiB |
| b77456b | 1.58 MiB | 2.12 MiB | 548.11 KiB |
| 17a0955 | 1.58 MiB | 2.10 MiB | 533.20 KiB |
| 27d7cf8 | 1.58 MiB | 2.12 MiB | 549.42 KiB |
| b750b96 | 1.58 MiB | 2.10 MiB | 533.19 KiB |
| d217708 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
1c2c6f4 to
2e6cf74
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📜 Description
Introduce new data classes (
io.sentry.util.network pkg), following format of the javascript SDK.SentryOkHttpInterceptoruses NetworkDetailCaptureUtils to construct these 'Network Details' data classes for insertion into the Breadcrumb Hint (name="sentry:replayNetworkDetails")).Initalise
DefaultReplayBreadcrumbConverteras the SDK-wide BeforeBreadcrumbCallback in order to capture the Network Details data from the Breadcrumb hint (placed there by SentryOkHttpInterceptor).Later when converting Breadcrumb to RRWebSpanEvent,
DefaultReplayBreadcrumbConverterlooks up capturedNetwork Detailsdata for insertion into the replayperformanceSpanwhen creating the replay segment.Implementation
Current impl handles JSON, UrlFormEncoded, skips these binary content types, and falls back to raw String as possible. Notably, it does not handle xml bodies (treats them as plaintext).
Follow-ups
networkDetail*SDK Options flags Will publish SDK Options in a Future PR along with changelog #skip-changeloghandle case where SentryOkHttpEventListener handles http request instrumentation Will handle in a follow-up PR
More unit tests after addressing the PR feedback, e.g. SentryOkHttpInterceptor, AndroidOptionsInitializer, NetworkBodyParser (at least the json part)
💡 Motivation and Context
Part of [Mobile Replay] Capture Network request/response bodies
Initially, we were trying to keep SDK changes simple and re-use the existing OKHTTP_REQUEST|RESPONSE hint data.
However, the
:sentry-android-replaygradle module doesn't compile against any of the http libs (makes sense).Because these
okhttp3.Request, etc types don't exist in:sentry-android-replay, replay accesses the NetworkDetails data via an abstracted network details data object ("sentry:replayNetworkDetails") on the Breadcrumb. When the SDKOptions constraints have been met.💚 How did you test it?
this replay session is an example recording with no
networkDetail*SDK options enabled(replay sessions below were captured without commit 4bc5b7714).
recording segment data shows sample replay payload .
replay session respects networkDetail[Request|Response]Headers
https://sentry-sdks.sentry.io/explore/replays/f89535ea0e79499ca8d75e34e6270925
replay session captures GET request bodies as null
replay session

replay session captures JSON bodies
1. Request body formatted as JSON key/values

ref
2. Response body formatted as JSON key/values

ref
replay session captures plaintext bodies
replay session

replay session captures form bodies (formurlencoded)
replay session

replay session captures binary bodies
1. binary body under size limit (summarized body content
https://sentry-sdks.sentry.io/explore/replays/830c3fa5675846b09575ed9ba06a6416

2. binary body over size limit (invalid response size, summarized body content

https://sentry-sdks.sentry.io/explore/replays/830c3fa5675846b09575ed9ba06a6416
replay session truncates plaintext bodies over 150KB (`MAX_NETWORK_BODY_SIZE`)
https://sentry-sdks.sentry.io/explore/replays/830c3fa5675846b09575ed9ba06a6416

replay sessions properly capture one-shot okhttp request bodies
One-shot HTTP Request Body request

Test:
read(..)okhttp3.Request.body in an OkHttpInterceptor after cloning body into new request causes the request to succeed ✅ (current impl)replay session
Valid response body logged to sentry:

Sanity Test:
read(..)okhttp3.Request.body in an OkHttpInterceptor without cloning body into a new request causes the request to fail ✅replay session
Invalid response body logged to sentry:
